Skip to content

crypto: follow up on OpenSSL backend split#64547

Open
panva wants to merge 4 commits into
nodejs:mainfrom
panva:followup-crypto-split
Open

crypto: follow up on OpenSSL backend split#64547
panva wants to merge 4 commits into
nodejs:mainfrom
panva:followup-crypto-split

Conversation

@panva

@panva panva commented Jul 16, 2026

Copy link
Copy Markdown
Member

This follows up on #64211, which separated the OpenSSL >= 3, BoringSSL, and legacy OpenSSL backend paths.

It addresses issues identified during a subsequent review of that split (done with GPT-5.6 Sol). The changes restore unintentionally altered legacy behavior, improve handling of provider-backed keys, and ensure temporary private key material is cleansed appropriately.

Refs: #64211

Details

crypto: retain legacy DH validation

Limit the g >= p rejection needed by the provider backend to OpenSSL 3. Preserve the legacy DiffieHellman object and expose the validation result through verifyError as before.


crypto: handle incomplete RSA private keys

Treat missing private RSA parameters as an export failure instead of passing null BIGNUM pointers to the JWK encoder. Also stop constructing a usable RSA view when reading an optional parameter itself fails.


crypto: cleanse provider private key copies

Clear provider-exported RSA, EC, and DH private BIGNUMs before freeing them. Also cleanse OSSL_PARAM builder copies and the plaintext DER intermediate used for encrypted traditional PEM output.


crypto: preserve RSA-PSS legacy pubkey DER

Recreate the historical rsaEncryption AlgorithmIdentifier used by X509Certificate legacy pubkey output for restricted and unrestricted RSA-PSS keys without relying on deprecated RSA APIs.

@panva panva added crypto Issues and PRs related to the crypto subsystem. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Jul 16, 2026
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.24%. Comparing base (a27fe21) to head (74bb180).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_rsa.cc 66.66% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64547      +/-   ##
==========================================
- Coverage   90.24%   90.24%   -0.01%     
==========================================
  Files         739      739              
  Lines      241698   241699       +1     
  Branches    45556    45552       -4     
==========================================
- Hits       218129   218119      -10     
- Misses      15103    15107       +4     
- Partials     8466     8473       +7     
Files with missing lines Coverage Δ
src/crypto/crypto_dh.cc 67.16% <ø> (ø)
src/crypto/crypto_rsa.cc 64.11% <66.66%> (+0.05%) ⬆️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 17, 2026
@panva

panva commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

cc @nodejs/crypto

@panva
panva requested review from jasnell and richardlau July 17, 2026 06:19
@panva
panva force-pushed the followup-crypto-split branch from ecd623a to c5e29e7 Compare July 17, 2026 09:04
panva added 3 commits July 17, 2026 12:39
Limit the g >= p rejection needed by the provider backend to OpenSSL 3.
Preserve the legacy DiffieHellman object and expose the validation
result through verifyError as before.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Treat missing private RSA parameters as an export failure instead of
passing null BIGNUM pointers to the JWK encoder. Also stop constructing
a usable RSA view when reading an optional parameter itself fails.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Clear provider-exported RSA, EC, and DH private BIGNUMs before freeing
them. Also cleanse OSSL_PARAM builder copies and the plaintext DER
intermediate used for encrypted traditional PEM output.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva
panva force-pushed the followup-crypto-split branch from c5e29e7 to 32c32b8 Compare July 17, 2026 10:39
Recreate the historical rsaEncryption AlgorithmIdentifier used by
X509Certificate legacy pubkey output for restricted and unrestricted
RSA-PSS keys without relying on deprecated RSA APIs.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva
panva force-pushed the followup-crypto-split branch from 32c32b8 to 74bb180 Compare July 17, 2026 11:39
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants